-
Notifications
You must be signed in to change notification settings - Fork 134
Add document to configure Basic Auth #1634
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add document to configure Basic Auth #1634
Conversation
✅ Deploy Preview will be available once build job completes!
|
ADubhlaoich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Initial bit of metadata feedback - more feedback coming.
There's a lot of stuff to be changed.
salonichf5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me overall, just a couple of comments
…Verify Basic Authentication section
Thanks Alan! Appreciate you taking the time to look over it all 😄 |
salonichf5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To make this easier for users, let’s ensure every curl command is copy/paste ready and works exactly as written. I recommended some edits but it would be better if you cross check again on your end.
salonichf5
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good to me
sjberman
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One small update. Once other comments are addressed, lgtm!
ADubhlaoich
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM. Added a lot of edit suggestions for consistent language and tone.
My apologies for the delay in review.
| ## Setup | ||
|
|
||
| In this part of the document, we will set up several resources in your cluster to demonstrate usage of the AuthenticationFilter CRD. | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Setup | |
| In this part of the document, we will set up several resources in your cluster to demonstrate usage of the AuthenticationFilter CRD. |
The rest of the document only deals with one level of heading/section nesting, so this is unnecessary. Adding additional edit suggestions to "pop" the remainder one level up.
| curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/coffee -u user1:wrong | ||
| ``` | ||
|
|
||
| Response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Response: | |
| The response should look similar to the following: |
| </html> | ||
| ``` | ||
|
|
||
| Accessing `/tea` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Accessing `/tea` | |
| Access`/tea` with no credentials using `curl`: |
| curl --resolve cafe.example.com:$GW_PORT:$GW_IP http://cafe.example.com:$GW_PORT/tea | ||
| ``` | ||
|
|
||
| Response: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Response: | |
| The response should look similar to the following: |
| URI: /tea | ||
| Request ID: c7eb0509303de1c160cb7e7d2ac1d99f | ||
| ``` | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Events: <none> | ||
| ``` | ||
|
|
||
| ## Verify Basic Authentication |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ## Verify Basic Authentication | |
| ## Test the AuthenticationFilter responses |
Proposed changes
This change adds a new document to the NGF traffic security section, which details on to configure Basic Authentication for NGF using the AuthenticationFilter CR
Closes nginx/nginx-gateway-fabric#4345
Checklist
Before sharing this pull request, I completed the following checklist:
Footnotes
Potentially sensitive information includes personally identify information (PII), authentication credentials, and live URLs. Refer to the style guide for guidance about placeholder content. ↩